-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade to go 119 #522
Upgrade to go 119 #522
Conversation
closes #492 Signed-off-by: Evgenii Baidakov <[email protected]>
Signed-off-by: Evgenii Baidakov <[email protected]>
Signed-off-by: Evgenii Baidakov <[email protected]>
Can you, please, do the same as we did in the node repo? Is allows us to be sure this PR broke nothing in the other places. |
@@ -117,11 +117,14 @@ type clientStatusMonitor struct { | |||
} | |||
|
|||
func newClientStatusMonitor(addr string, errorThreshold uint32) clientStatusMonitor { | |||
return clientStatusMonitor{ | |||
m := clientStatusMonitor{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, i got through this in the node repo but that is the second time i see how adapting to sync
makes it harder to read and adds more lines. so maybe add some helper atomic constructor func to our repos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, no. Frankly, I've expected the stdlib sync
to be somewhat more friendly, but it's not that bad either. The best thing here is that it's a standard one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love keeping things as standard as they are so wont argue about that
I've expected the stdlib sync to be somewhat more friendly
but +1 here
Also, the first two commits are the same for me. But up to you. |
Signed-off-by: Evgenii Baidakov <[email protected]>
Original message is - copylocks: return copies lock value: github.com/nspcc-dev/neofs-sdk-go/pool.clientStatusMonitor contains sync.RWMutex. Signed-off-by: Evgenii Baidakov <[email protected]>
Signed-off-by: Evgenii Baidakov <[email protected]>
Version v0.6/v1 is out of support. V2 brings generics and makes out code a little bit easier. Signed-off-by: Evgenii Baidakov <[email protected]>
Signed-off-by: Evgenii Baidakov <[email protected]>
1fff15f
to
605f92c
Compare
Sometimes touching one line of code breaks the other and we're 100% clean now. Signed-off-by: Evgenii Baidakov <[email protected]>
Works perfectly https://github.com/nspcc-dev/neofs-sdk-go/actions/runs/6269928793/job/17027095101?pr=522 😅 Do we suppress these errors? Fixing them may significantly affect the SDK API |
As for deprecation warnings, you can suppress them if there are no better ways (although it's somewhat strange to have something deprecated and not have a replacement). |
Signed-off-by: Evgenii Baidakov <[email protected]>
Yes, maybe we should pay more attention to #25. But what about:
panic? Changing the function API to return an error may significantly influence some code. |
With the current API you can return |
Updated |
That is because of nspcc-dev/neofs-api#205. No need to store SG's expiration as a payload's part if it is always an object that has its own expiration. So maybe deprecate SDK's getters too? Or even remove them. @smallhive, @roman-khimov, @cthulhu-rider |
Signed-off-by: Evgenii Baidakov <[email protected]>
7f2aa36
to
db2d189
Compare
db2d189
to
5058cd1
Compare
5058cd1
to
0f77f94
Compare
Signed-off-by: Evgenii Baidakov <[email protected]>
0f77f94
to
96a0e41
Compare
Close #492